feat: adding count with filtering operations to OpenSearchDocumentStore
#2653
Conversation
github-actions
bot
added
integration:opensearch
type:documentation
davidsbatista
changed the title
davidsbatista
changed the title
OpenSearchDocumentStore
|
Hey @tstadel I'd also appreciate your review on this since we want to make sure it will in platform as well. |
integrations/opensearch/src/haystack_integrations/document_stores/opensearch/document_store.py
Outdated
Show resolved
Hide resolved
…res/opensearch/document_store.py Co-authored-by: Sebastian Husch Lee <[email protected]>
…res/opensearch/document_store.py Co-authored-by: Sebastian Husch Lee <[email protected]>
…res/opensearch/document_store.py Co-authored-by: Sebastian Husch Lee <[email protected]>
…res/opensearch/document_store.py Co-authored-by: Sebastian Husch Lee <[email protected]>
…res/opensearch/document_store.py Co-authored-by: Sebastian Husch Lee <[email protected]>
…res/opensearch/document_store.py Co-authored-by: Sebastian Husch Lee <[email protected]>
integrations/opensearch/src/haystack_integrations/document_stores/opensearch/document_store.py
Outdated
Show resolved
Hide resolved
integrations/opensearch/src/haystack_integrations/document_stores/opensearch/document_store.py
Outdated
Show resolved
Hide resolved
integrations/opensearch/src/haystack_integrations/document_stores/opensearch/document_store.py
Outdated
Show resolved
Hide resolved
…res/opensearch/document_store.py Co-authored-by: Sebastian Husch Lee <[email protected]>
…res/opensearch/document_store.py Co-authored-by: Sebastian Husch Lee <[email protected]>
| This method would return: | ||
| { | ||
| 'content': {'type': 'text'}, | ||
| 'category': {'type': 'keyword'}, | ||
| 'status': {'type': 'keyword'}, | ||
| 'priority': {'type': 'long'}, | ||
| } |
There was a problem hiding this comment.
It seems like the return type of this function get_metadata_fields_info will be specific to OpenSearch. Whereas all other functions you've added that we are looking to add across all Document Stores have a format we created.
Do you think it's possible or reasonable for us to try and standardize something like this?
There was a problem hiding this comment.
We can add a transformation inside each DocumentStore get_metadata_fields_info so that it returns in this format. When this PR is merged I will adapt the issues: noting this + renaming the function names and signature.
| assert len(draft_docs) == 1 | ||
| assert draft_docs[0].meta["category"] == "B" | ||
|
|
||
| def test_count_documents_by_filter(self, document_store: OpenSearchDocumentStore): |
There was a problem hiding this comment.
Probably out of scope for this PR, but could we open an issue to move these standardized tests into haystack.testing.document_store as testing modules? I think this would help reduce the duplicate test code we have in all of our document store integrations.
WDYT?
There was a problem hiding this comment.
good catch yes, that's a good idea 👍🏽
There was a problem hiding this comment.
The only issue I'm seeing with this is the parameters of the tests when tied to a specific DocStore. We probably can extend the DocStore Protocol and add a few more operations, if all of our document stores already support them.
Related Issues
OpenSearchDocumentStore#2635Proposed Changes:
count_documents_by_filter()- count documents matching filter criteriacount_distinct_values_by_filter()- get distinct value counts for metadata fields with optional filteringget_fields_info()- retrieve field type information from index mappingget_field_min_max()- get min/max values for numeric metadata fieldsget_field_unique_values()- get unique values for a field with pagination and content-based filteringquery_sql()- execute SQL queries against OpenSearch with support for multiple response formats (JSON, CSV, JDBC, RAW)How did you test it?
Notes for the reviewer
httpx>=0.28.1dependencyChecklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:.